Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change network id's to be below 109 #495

Merged

Conversation

petong
Copy link
Contributor

@petong petong commented Aug 1, 2018

I think this should do it

@petong petong requested review from pgebheim and bthaile August 1, 2018 21:53
Copy link
Member

@pgebheim pgebheim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will need to redeploy to the pop containers after this is merged. @epheph

@epheph
Copy link
Contributor

epheph commented Aug 1, 2018

This is a step in the right direction (maybe? It's not a step in the wrong direction, for sure), as it will change the chainId, because we set the networkId and chainId to the same value in our geth nodes. It will not fully fix our issue, though, as chainId (for some insane reason) is NOT queryable from the eth node (😭 🤷‍♂️ ).
So somewhere in our transaction-creation libraries, there must be some static mapping of networkId to chainId in the ethereum client ? And we'll need to add 101=101, 102=102, etc there.

An EIP to add chainId is merged but not implemented: ethereum/EIPs#694

@petong petong merged commit f1594e2 into master Aug 2, 2018
@pgebheim pgebheim deleted the petong/ch14845/update-networkid-for-locally-hosted-docker branch August 15, 2018 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants